-
-
Notifications
You must be signed in to change notification settings - Fork 5
[wip] additional template customizations #406
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea looks good. I've done updated the Java implementation with a builder and Javascript and JS implementations with only default/blank values in #409.
Though I've got three conflicts
- I added a
{{custom_script}}
to the template - You have a
{{custom-head}}
in the template. - We picked different naming conventions for template variable names.
Also some remarks below.
public class HtmlReportSettings | ||
{ | ||
private const string DEFAULT_TITLE = "Cucumber"; | ||
private const string DEFAULT_ICON = "data:image/svg+xml;base64,PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHZpZXdCb3g9IjAuMDYgMC41NiAzMi41IDM3LjEzIj4KICAgIDxnIGZpbGw9Im5vbmUiIGZpbGwtcnVsZT0iZXZlbm9kZCI+CiAgICAgICAgPHBhdGggZD0iTS00LTFoNDB2NDBILTR6Ii8+CiAgICAgICAgPHBhdGggZmlsbD0iIzAwYTgxOCIKICAgICAgICAgICAgICBkPSJNMTYuNDM4LjU2M0M3LjM4Ni41NjMuMDYzIDcuODg2LjA2MyAxNi45MzhjMCA3Ljk2OCA1LjcxMiAxNC41ODkgMTMuMjUgMTYuMDYydjQuNjg4YzkuOC0xLjQ3OCAxOC40NzctOS4yNTcgMTkuMTI0LTE5LjQ3LjM5LTYuMTQ2LTIuNjc0LTEyLjQyMS03Ljg0My0xNS40NjhhMTMuNjIgMTMuNjIgMCAwIDAtMS44NzUtLjkzOGwtLjMxMy0uMTI1Yy0uMjg3LS4xMDYtLjU3Ny0uMjI1LS44NzUtLjMxMmExNi4yNDYgMTYuMjQ2IDAgMCAwLTUuMDkzLS44MTN2LjAwMXoiLz4KICAgICAgICA8cGF0aCBmaWxsPSIjZmZmIgogICAgICAgICAgICAgIGQ9Ik0xOS44MTMgNi42MjVhMS43ODcgMS43ODcgMCAwIDAtMS41NjMuNjI1Yy0uMy40LS40ODguNzg3LS42ODggMS4xODgtLjYgMS40LS40IDIuOS41IDQgMS40LS4zIDIuNTg4LTEuMTk0IDMuMTg4LTIuNTk0LjItLjQuMzEzLS45MTMuMzEzLTEuMzEzLjA2Mi0xLjA2Mi0uODE3LTEuODEtMS43NS0xLjkwNnptLTcuMjgyLjA5NGMtLjkxMy4wODctMS43ODEuODEyLTEuNzgxIDEuODEyIDAgLjQuMTEzLjkxMy4zMTMgMS4zMTMuNiAxLjQgMS44OCAyLjI5MyAzLjI4IDIuNTk0LjgtMS4xIDEuMDA3LTIuNi40MDctNC0uMi0uNC0uMzg3LS43OTQtLjY4OC0xLjA5NGExLjc1NyAxLjc1NyAwIDAgMC0xLjUzLS42MjVoLS4wMDF6TTcuNjI1IDExLjUzYy0xLjU3Ny4wODEtMi4yODEgMi4wNjMtLjk2OSAzLjA5NC40LjMuNzg4LjUxOSAxLjE4OC43MTkgMS40LjYgMy4wMTkuMzk0IDQuMjE4LS40MDYtLjMtMS4zLTEuMzE4LTIuNDk0LTIuNzE4LTMuMDk0LS41LS4yLS45MDYtLjMxMy0xLjQwNi0uMzEzLS4xMTMtLjAxMi0uMjA4LS4wMDUtLjMxMyAwem0xNS40MDYgNi4wNjNhNC41NzQgNC41NzQgMCAwIDAtMi41OTMuNzVjLjMgMS4zIDEuMzE4IDIuNDkzIDIuNzE4IDMuMDkzLjUuMi45MDcuMzEzIDEuNDA3LjMxMyAxLjguMSAyLjY4LTIuMTI1IDEuMjgtMy4xMjUtLjQtLjMtLjc4Ny0uNDg4LTEuMTg3LS42ODhhNC4zMiA0LjMyIDAgMCAwLTEuNjI1LS4zNDN6bS0xMy42NTYuMDkzYy0uNTUuMDExLTEuMS4xMi0xLjYyNS4zNDQtLjUuMi0uODg4LjQxOS0xLjE4OC43MTktMS4zIDEuMS0uNDI1IDMuMTk0IDEuMzc1IDMuMDk0LjUgMCAxLjAwNy0uMTEzIDEuNDA3LS4zMTMgMS40LS42IDIuMzk0LTEuNzkzIDIuNTk0LTMuMDkzYTQuNDc1IDQuNDc1IDAgMCAwLTIuNTYzLS43NXYtLjAwMXptNS4wNjMgMy4wNjNjLTEuNC4zLTIuNTg4IDEuMTk0LTMuMTg4IDIuNTk0LS4yLjQtLjMxMy44ODEtLjMxMyAxLjI4MS0uMSAxLjcgMi4yMiAyLjYxMyAzLjIyIDEuMzEzLjMtLjQuNDg3LS43ODguNjg3LTEuMTg4LjYtMS4zLjM5NC0yLjgtLjQwNi00em0zLjcxOC4wOTRjLS44IDEuMS0xLjAwNiAyLjYtLjQwNiA0IC4yLjQuMzg3Ljc5My42ODggMS4wOTMgMS4xIDEuMiAzLjQxMi4zMTMgMy4zMTItMS4xODcgMC0uNC0uMTEzLS45MTMtLjMxMy0xLjMxMy0uNi0xLjQtMS44OC0yLjI5My0zLjI4LTIuNTkzaC0uMDAxeiIvPgogICAgPC9nPgo8L3N2Zz4K"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be extracted to a file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in #409. Just has to be read from a file.
<style> | ||
{{css}} | ||
{{custom-css}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be in a separate style tag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mustash naming conventions are lower underscore case.
await WriteTemplateBetweenAsync(_writer, processedTemplate, SCRIPT_MARKER, null); | ||
} | ||
|
||
private string ApplySettingsToTemplate(string template) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this method can be replaced with a sequence of WriteTemplateBetweenAsync
calls.
Either:
- The template is effectively a constant, so we don't have to check if it a template variable should be replaced.
Or:
- The template is not a constant, then the repeated
.Replace
calls are prone to accidental injection. For example the title is set to{{script}}
.
So I think it would be better to consider the template to be constant. The alternative leads to problems that can only be solved by using a mustache template engine which we are trying to avoid to keep the dependencies small.
edit: I'm actually okay if you want to use a template engine. But would be good to do that explicitly and not reinvent the wheel too much. π
<meta content="text/html;charset=utf-8" http-equiv="Content-Type"> | ||
<meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
<link rel="icon" href="data:image/svg+xml;base64,PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHZpZXdCb3g9IjAuMDYgMC41NiAzMi41IDM3LjEzIj4KICAgIDxnIGZpbGw9Im5vbmUiIGZpbGwtcnVsZT0iZXZlbm9kZCI+CiAgICAgICAgPHBhdGggZD0iTS00LTFoNDB2NDBILTR6Ii8+CiAgICAgICAgPHBhdGggZmlsbD0iIzAwYTgxOCIKICAgICAgICAgICAgICBkPSJNMTYuNDM4LjU2M0M3LjM4Ni41NjMuMDYzIDcuODg2LjA2MyAxNi45MzhjMCA3Ljk2OCA1LjcxMiAxNC41ODkgMTMuMjUgMTYuMDYydjQuNjg4YzkuOC0xLjQ3OCAxOC40NzctOS4yNTcgMTkuMTI0LTE5LjQ3LjM5LTYuMTQ2LTIuNjc0LTEyLjQyMS03Ljg0My0xNS40NjhhMTMuNjIgMTMuNjIgMCAwIDAtMS44NzUtLjkzOGwtLjMxMy0uMTI1Yy0uMjg3LS4xMDYtLjU3Ny0uMjI1LS44NzUtLjMxMmExNi4yNDYgMTYuMjQ2IDAgMCAwLTUuMDkzLS44MTN2LjAwMXoiLz4KICAgICAgICA8cGF0aCBmaWxsPSIjZmZmIgogICAgICAgICAgICAgIGQ9Ik0xOS44MTMgNi42MjVhMS43ODcgMS43ODcgMCAwIDAtMS41NjMuNjI1Yy0uMy40LS40ODguNzg3LS42ODggMS4xODgtLjYgMS40LS40IDIuOS41IDQgMS40LS4zIDIuNTg4LTEuMTk0IDMuMTg4LTIuNTk0LjItLjQuMzEzLS45MTMuMzEzLTEuMzEzLjA2Mi0xLjA2Mi0uODE3LTEuODEtMS43NS0xLjkwNnptLTcuMjgyLjA5NGMtLjkxMy4wODctMS43ODEuODEyLTEuNzgxIDEuODEyIDAgLjQuMTEzLjkxMy4zMTMgMS4zMTMuNiAxLjQgMS44OCAyLjI5MyAzLjI4IDIuNTk0LjgtMS4xIDEuMDA3LTIuNi40MDctNC0uMi0uNC0uMzg3LS43OTQtLjY4OC0xLjA5NGExLjc1NyAxLjc1NyAwIDAgMC0xLjUzLS42MjVoLS4wMDF6TTcuNjI1IDExLjUzYy0xLjU3Ny4wODEtMi4yODEgMi4wNjMtLjk2OSAzLjA5NC40LjMuNzg4LjUxOSAxLjE4OC43MTkgMS40LjYgMy4wMTkuMzk0IDQuMjE4LS40MDYtLjMtMS4zLTEuMzE4LTIuNDk0LTIuNzE4LTMuMDk0LS41LS4yLS45MDYtLjMxMy0xLjQwNi0uMzEzLS4xMTMtLjAxMi0uMjA4LS4wMDUtLjMxMyAwem0xNS40MDYgNi4wNjNhNC41NzQgNC41NzQgMCAwIDAtMi41OTMuNzVjLjMgMS4zIDEuMzE4IDIuNDkzIDIuNzE4IDMuMDkzLjUuMi45MDcuMzEzIDEuNDA3LjMxMyAxLjguMSAyLjY4LTIuMTI1IDEuMjgtMy4xMjUtLjQtLjMtLjc4Ny0uNDg4LTEuMTg3LS42ODhhNC4zMiA0LjMyIDAgMCAwLTEuNjI1LS4zNDN6bS0xMy42NTYuMDkzYy0uNTUuMDExLTEuMS4xMi0xLjYyNS4zNDQtLjUuMi0uODg4LjQxOS0xLjE4OC43MTktMS4zIDEuMS0uNDI1IDMuMTk0IDEuMzc1IDMuMDk0LjUgMCAxLjAwNy0uMTEzIDEuNDA3LS4zMTMgMS40LS42IDIuMzk0LTEuNzkzIDIuNTk0LTMuMDkzYTQuNDc1IDQuNDc1IDAgMCAwLTIuNTYzLS43NXYtLjAwMXptNS4wNjMgMy4wNjNjLTEuNC4zLTIuNTg4IDEuMTk0LTMuMTg4IDIuNTk0LS4yLjQtLjMxMy44ODEtLjMxMyAxLjI4MS0uMSAxLjcgMi4yMiAyLjYxMyAzLjIyIDEuMzEzLjMtLjQuNDg3LS43ODguNjg3LTEuMTg4LjYtMS4zLjM5NC0yLjgtLjQwNi00em0zLjcxOC4wOTRjLS44IDEuMS0xLjAwNiAyLjYtLjQwNiA0IC4yLjQuMzg3Ljc5My42ODggMS4wOTMgMS4xIDEuMiAzLjQxMi4zMTMgMy4zMTItMS4xODcgMC0uNC0uMTEzLS45MTMtLjMxMy0xLjMxMy0uNi0xLjQtMS44OC0yLjI5My0zLjI4LTIuNTkzaC0uMDAxeiIvPgogICAgPC9nPgo8L3N2Zz4K"> | ||
<link rel="icon" href="{{icon}}"> | ||
{{custom-head}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the purpose of this element?
Closing in favor of #409 |
π€ What's changed?
β‘οΈ What's your motivation?
implement #403
π·οΈ What kind of change is this?
β»οΈ Anything particular you want feedback on?
π Checklist:
This text was originally generated from a template, then edited by hand. You can modify the template here.